Improve Border Router packet forwarding logic #97531
Conversation
zephyrbot
requested review from
canisLupus1313,
edmont,
jukkar,
maciejbaczmanski,
pdgendt,
rlubos and
ssharks
Cristib05
force-pushed
the
enhance_border_router_packet_forwarding
branch
from
4524f47 to
3d4abde
Compare
| "ff04::1", | ||
| /** Site-Local scope multicast address */ | ||
| "ff05::1", | ||
| /** Organization-Local scope multicast address */ | ||
| "ff08::1", | ||
| /** Global scope multicast address */ | ||
| "ff0e::1" |
There was a problem hiding this comment.
This looks like overkill as you could create IPv6 address dynamically.
So something like
int mcast_groups[] = {
/** Admin-Local scope multicast address */
4,
/** Site-Local scope multicast address */
5,
/** Organization-Local scope multicast address */
8,
/** Global scope multicast address */
e,
};
ARRAY_FOR_EACH(mcast_groups, i) {
struct in6_addr addr;
net_ipv6_addr_create(&addr, (0xff << 8) | i, 0, 0, 0, 0, 0, 0, 1);
...
}
3d4abde to
08b9ae7
Compare
Cristib05
force-pushed
the
enhance_border_router_packet_forwarding
branch
from
08b9ae7 to
141cd57
Compare
| static int mcast_group_idx[] = {/** Admin-Local scope multicast address */ | ||
| 0x04, | ||
| /** Site-Local scope multicast address */ | ||
| 0x05, | ||
| /** Organization-Local scope multicast address */ | ||
| 0x08, | ||
| /** Global scope multicast address */ | ||
| 0x0e, | ||
| }; |
There was a problem hiding this comment.
Just wondering the layout of this and why indent like that. Are you using clang-format for this? Asking because it looks weird to add comment right after starting block {, also makes line quite long.
There was a problem hiding this comment.
I have clang-format 20.1.5 and I use scripts/ci/check_compliance.py. I created a new commit where I have moved the first comment to the next line. This was the output:
There was a problem hiding this comment.
That is very weird indeed, try putting the comment on the same line?
static int mcast_group_idx[] = {
0x04, /** Admin-Local scope multicast address */
0x05, /** Site-Local scope multicast address */
0x08, /** Organization-Local scope multicast address */
0x0e, /** Global scope multicast address */
};There was a problem hiding this comment.
Modified as per last example, no clang-format issues on my side, hopefully we have a winner :)
Cristib05
force-pushed
the
enhance_border_router_packet_forwarding
branch
from
141cd57 to
709f956
Compare
There are some cases when OpenThread opens a sockets and doesn't choose as default it's internal interface, this leading to usage of platform UDP module which will then send back the packet to the OpenThread interface. In this case, the packet should not be treated as originated from backbone interface. Backbone router multicast listener callback functionality is improved. A route with a prefix length of 128 is set and a multicast address is added for each listener registration. OpenThread interface joins that multicast address group. Enabled forwarding capabilities for Backbone interface. A border router should be able to perform default packet forwarding for destination addresses with a multicast scope greater than admin-local. In order to achieve this, multicast routes have been added to those addreses. [https://datatracker.ietf.org/doc/rfc7346/] For Border Router application, `ip6_addr_cb` is not installed. otIp6SubscribeMulticastAddress call would re-register an IPV6 multicast address which might have been registered by an OpenThread node using `ipmadd add` command and even if that node performed `ipmaddr del`, the address was still present in multicast listener table. This also led to a missing MLDv2 message with that specific multicast IPV6 address. Signed-off-by: Cristian Bulacu <[email protected]>
Cristib05
force-pushed
the
enhance_border_router_packet_forwarding
branch
from
709f956 to
30a61b4
Compare
| 0x04, /** Admin-Local scope multicast address */ | ||
| 0x05, /** Site-Local scope multicast address */ | ||
| 0x08, /** Organization-Local scope multicast address */ | ||
| 0x0e, /** Global scope multicast address */ |
There was a problem hiding this comment.
Unfortunately we need to have one more round. If you use this style, then the comment needs to be written with < so like this
0x04, /**< Admin-Local scope multicast address */
0x05, /**< Site-Local scope multicast address */
0x08, /**< Organization-Local scope multicast address */
0x0e, /**< Global scope multicast address */
otherwise the comment in documentation refers to the next line.
There was a problem hiding this comment.
Is this even parsed for documentation?
There was a problem hiding this comment.
Hmm, very good point. In that case we could just say /* xxxx */ without doxygen comment. If we keep doxygen comment, and want to document this, then we should do /**< xxxx */.
Anyway, as this is in .c file, it does not matter much, I was a bit too pedantic here.
|
5 participants
There are some cases when OpenThread opens a sockets and doesn't choose as default it's internal interface, this leading to usage of platform UDP module which will then send back the packet to the OpenThread interface. In this case, the packet should not be treated as originated from backbone interface.
Backbone router multicast listener callback functionality is improved. A route with a prefix length of 128 is set
and a multicast address is added for each listener registration. OpenThread interface joins that multicast address group.
Enabled forwarding capabilities for Backbone interface. A border router should be able to perform default packet forwarding for destination addresses with a multicast scope greater than admin-local. In order to achieve this, multicast routes have been added to those addreses. [https://datatracker.ietf.org/doc/rfc7346/]
For Border Router application,
ip6_addr_cbis not installed. otIp6SubscribeMulticastAddress call would re-register an IPV6 multicast address which might have been registered by an OpenThread node usingipmadd addcommand and even if that node performedipmaddr del, the address was still present in multicast listener table. This also led to a missing MLDv2 message with that specific multicast IPV6 address.